commit by spirinamayya · Pull Request #1 · spirinamayya/avltree

spirinamayya · 2023-04-26T19:11:26Z

No description provided.

DimaTrushin

Есть несколько общих слов.

добавить .gitignore
убрать .DS_Store файл
подключить clang-format для форматирования кода

В целом код приятно читается, но есть ошибки проектирования и работы с плюсами, особенно утечки в Qt. Я постарался везде пройтись комментариями. Если ошибка повторялась, я не комментировал повторно. Я предполагаю, что ты по аналогии исправишь все нужные места.

DimaTrushin · 2023-04-30T17:52:23Z

Connection.cpp

+    _controller.ptr = &_model;
+    _view.subscribe(_controller.input());
+    _model.subscribe(_view.input());
+}


Не надо радовать пользователя гит репозитория "кирпичами". Делай пустую строку в конце каждого файла.

DimaTrushin · 2023-04-30T18:26:43Z

main.cpp

+    MainWindow w;
+    Connection c(&w);
+    w.show();


Вот это все выглядит как "кишки" единого объекта Application.
-Во-первых, у тебя раскиданы разные части по main, которые делают случайную работу, это нарушение уровня абстракции.

Во-вторых, я бы видел эту часть так. Надо создать объект application, который в себе содержит все нужные ресурсы и в своем конструкторе их инициализирует. Например так

class Application { public: private: Model model_; View view_; Controller controller_; };

При этом MainWindow должна быть частью View или должна выдать View необходимые ресурсы для работы.

В-третьих, у тебя совсем не понятна роль Connection.

Функция show() должна бызываться в конструкторе view.

DimaTrushin · 2023-04-30T18:27:48Z

Connection.h

+    //~Connection();
+
+private:
+    AVLTree _model;


Не делай _ в начале имен. Дело в том, что имена начинающиеся с _ или __ зарезервированы по стандарту (в данном случае проблем нет, но очень легко ошибиться). Потому лучше сделать их в конце.

DimaTrushin · 2023-04-30T18:28:13Z

Connection.h

+
+#include "Controller.h"
+
+class Connection{


Вот этот объект надо сделать Application, а MainWindow запихнуть во view.

DimaTrushin · 2023-04-30T18:28:30Z

Connection.h

+#define COURSEPROJECT_CONNECTION_H
+
+#include "Controller.h"
+


Помести весь свой проект в namespace. Не пиши ничего в global namespace.

DimaTrushin · 2023-04-30T19:33:32Z

Model.h

+
+//    std::vector<Node*> root_;
+    Node* root_ = nullptr;
+    int Value;


Вот это что такое? Зачем в дереве какое-то value дополнительно? Кроме того, ты имена переменных пишешь то с большой то с маленькой буквы. Надо определиться и выбрать единый стиль.

DimaTrushin · 2023-04-30T19:34:39Z

Model.cpp

+    insertPort.notify();
+}
+
+void AVLTree::clearHelp(Node*& node)


Функции в cpp файле должны идти в том же порядке, в каком они объявлены в header-е. Иначе невозможно искать нужную функцию.

DimaTrushin · 2023-04-30T19:37:52Z

Model.cpp

+    int l = 0, r = 0;
+    if(left != nullptr)
+        l = left->height;


Когда надо обрабатывать случай нулевого указателя, вместо вот такой ручной проверки, лучше сделать функцию внутри дерева, которая обрабатывает все случаи, например

static int getHeight(const Node* node) { if (node == nullptr) return 0; return node->height; }

И тогда можно будет просто написать

static int findMaxHeight(const Node* left, const Node* right) { return std::max(getHeight(left), getHeight(right)); }

DimaTrushin · 2023-04-30T19:39:00Z

Model.cpp

+
+
+Node* AVLTree::insert(Node* node, int key)
+{


Очень похоже, что в этом случае ты можешь разбить эту функцию на подфункции с теми именами, которые лежат в комментариях.

DimaTrushin · 2023-04-30T19:40:55Z

Model.cpp

+
+    if(root_== nullptr)
+        return;
+    insertPort.notify();


Если я правильно понял, то у тебя нет анимации.

DimaTrushin

В целом неплохо.
Потерян Model.h файл.
Еще есть несколько утечек во View.
И мне не нравится как задизайнен обход дерева, но видимо это надо так и оставить.

DimaTrushin · 2023-05-23T13:50:51Z

Controller.cpp

+    }

+    void Controller::action(const ViewData &data) {
+        if(data.operation == View::Operation::Add)


Я бы сделал switch по enum-у

DimaTrushin · 2023-05-23T13:51:27Z

Controller.cpp

+        else if(data.operation == View::Operation::Search)
+            model_->search(data.value);
+        else if(data.operation == View::Operation::Traversal && data.type == View::Type::InOrder)
+            model_->inOrder();


А что это за команда у дерева? Это чисто для визуацилации или это какой-то запрос у дерева, который оно выполняет?

DimaTrushin · 2023-05-23T13:54:10Z

InfoTree.h

+        Info* copy(const Node* node);
+        void calcYCoord(const Node* node);
+
+        static constexpr int RADIUS = 40;


Не пиши только большими буквами, так обычно пишут макросы, если нужны константы, то добавь k в начале, например kRadius или k_radius в зависимости от стиля. И можно _ в конце имени для единообразия.

DimaTrushin · 2023-05-23T13:57:32Z

InfoTree.cpp

+    }
+
+    void InfoTree::calcYCoord(const Node *rootGet) {
+        root_ = copy(rootGet);


Вынеси эту строчку в список инициализации в конструкторе

InfoTree::InfoTree() : root_(copy(rootGet)) { calcYCoord(); calcXCoord(); }

Дело в том, что эта функция копирования не имеет отношения к вычислению координат, она строит дерево. Ее еще стоит переименовать в buildInfoTree или что-то такое.

DimaTrushin · 2023-05-23T13:58:49Z

InfoTree.cpp

+        std::queue<Info *> que;
+        que.push(root);


Это ты от рекурсии избавлялась?

DimaTrushin · 2023-05-23T14:32:13Z

Model.cpp

Не могу найти Model.h файл.

DimaTrushin · 2023-05-23T14:34:42Z

Model.cpp

+        port_.notify();
+    }
+
+    void AVLTree::inOrder() {


Не лучшее решение. Вообще говоря, не понятно что такое эта операция для дерева.
Если это вопрос анимации, то это должно делаться на уровне InfoTree. А так что эта операция делает с деревом? Наверное сейчас уже лучше так и оставить, но в целом это ошибка дизайна.

DimaTrushin · 2023-05-23T14:37:00Z

Model.cpp

+        return cur;
+    }
+
+    void AVLTree::twoChildren(Node* node)


Вот это непонятная функция. У нее в названии нет глагола. Что она делает?

DimaTrushin · 2023-05-23T14:37:20Z

Model.cpp

+        node->rightCh = deleteNode(node->rightCh, cur->key);
+    }
+
+    Node* AVLTree::oneZeroChildren(Node* node)


Вот тут тоже не понятно, что функция делает, нет глагола.

DimaTrushin · 2023-05-23T14:40:20Z

mainwindow.ui

+    </widget>
+    <pixmapfunction/>
+    <connections/>
+</ui>


кирпич

commit

9a4ceb5

spirinamayya requested a review from DimaTrushin April 28, 2023 16:00

DimaTrushin requested changes Apr 30, 2023

View reviewed changes

update

f732c61

spirinamayya force-pushed the master branch from 2b59c8a to f732c61 Compare May 22, 2023 12:19

spirinamayya added 3 commits May 22, 2023 15:28

Ds_store

df90803

idea

a133817

idea

1726d0e

DimaTrushin requested changes May 23, 2023

View reviewed changes

step back

94167fc

spirinamayya force-pushed the master branch from fb1a8e8 to 94167fc Compare June 7, 2023 08:44

spirinamayya and others added 4 commits June 9, 2023 22:31

final

2198964

Create README.md

48f0ba6

Add files via upload

f3e3f12

Update README.md

0f95f34

		#define COURSEPROJECT_CONNECTION_H

		#include "Controller.h"

Conversation

spirinamayya commented Apr 26, 2023

Uh oh!

DimaTrushin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DimaTrushin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants